-
Notifications
You must be signed in to change notification settings - Fork 14
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Résolution de soucis mineurs d'héritage de classes #616
Conversation
def step(self) -> HabilitationFormStep: | ||
raise NotImplementedError() | ||
|
||
class OnlyNewRequestsView(HabilitationStepMixin, LateStageRequestView): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Comme ces vues concernent seulement des brouillons, on a forcément un formulaire donc on s'attend à un fil d'Ariane => on peut hériter directement du habilitationstepmixin au lieu de dupliquer la logique
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hmm du coup l'IDE râle qu'on n'implémente pas la méthode abstraite. Pas moyen de déclarer que la classe est abstraite sans dupliquer le step
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Bah l'IDE propoe aussi de rajouter abc.ABC
aux classes héritées. Mais je sais pas trop ce que c'est ni comment ça fonctionne alors j'ai évité.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Après, ça sert à rien de multiplier les classes abstraites à 1 méthode. Moi mon but à la base c'était surtout d'avoir 1 FormView
commune pour presque toutes les vues de l'application et éviter qu'on fasse des erreurs bêtes en modifiant, genre oublier de remettre le step
dans le contexte et péter le rail sans s'en rendre compte. Mais le code actuel fait plus vraiment ça donc je sais pas 🤔
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Le problème à la base c'est qu'on est loin d'avoir un fil d'ariane dans toutes les view, donc à mon avis ça n'avait pas grand intérêt de sortir un mixin pour ça.
Par contre l'héritage a de l'intérêt car plus on avance dans le process, plus on vérifie de choses (le demandeur, puis son mail, plus le statut de la demande, etc.) et on n'a pas envie de dupliquer la vérif. Côté aidants_connect_web on gérait ça avec des décorateurs et ça tournait bien je trouve, mais je ne sais pas si c'est possible avec des CBV ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
les CBV sont globalement un nids a mixin :)
C'est même parfois un reproche qui leur ait fait.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
par contre j'ai pas fini mon deuxieme café, je viens de voir que le sujet était les décorateurs !
c'est aussi possible sur les CBV, un peu plus compliqué que juste sur des fonctions, mais oui ça se fait
@@ -79,22 +79,14 @@ def dispatch(self, request, *args, **kwargs): | |||
|
|||
|
|||
class LateStageRequestView(VerifiedEmailIssuerView, View): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ces vues peuvent concerner des formulaires ou non, donc pas forcément besoin de fil d'Ariane => pas de step attendu, pas d'héritage de habilitationstep
@@ -287,7 +279,7 @@ def get_form_kwargs(self): | |||
return {**super().get_form_kwargs(), "instance": self.organisation} | |||
|
|||
|
|||
class PersonnelRequestFormView(OnlyNewRequestsView, HabilitationStepMixin, FormView): | |||
class PersonnelRequestFormView(OnlyNewRequestsView, FormView): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Et ici et sur la suivante, comme OnlyNewRequestsView hérite de HabilitationStepMixin, pas besoin de le rappeler ici
1a9c935
to
11df410
Compare
🌮 Objectif
Éviter des avertissements dans les IDE
🔍 Implémentation
step